v3: replace go-git direct dependency with system git CLI#5520
Open
leaanthony wants to merge 1 commit into
Open
v3: replace go-git direct dependency with system git CLI#5520leaanthony wants to merge 1 commit into
leaanthony wants to merge 1 commit into
Conversation
Introduces v3/internal/git, a thin stdlib-only wrapper around the system git executable, covering HeadHash, Clone, Init, RemoteAdd and AddAll. All five callers that previously imported go-git/go-git/v5 are migrated to use this package. go-git/go-git/v5 moves from a direct to an indirect dependency in go.mod (it is still pulled in transitively by wailsapp/task/v3). When git is not found in PATH, all functions return git.ErrNotInstalled with a link to https://git-scm.com so callers can surface a clear message to users. 100% statement coverage via v3/internal/git/git_test.go.
Contributor
WalkthroughThis PR replaces the project's direct dependency on ChangesGo-git Library Replacement
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces direct use of go-git in v3 code paths with a new stdlib-based wrapper around the system git CLI, reducing the direct dependency surface while making Git an external runtime requirement.
Changes:
- Adds
v3/internal/gitwith CLI-backed helpers for clone, init, remote add, add all, and HEAD hash lookup. - Migrates init, templates, doctor, doctor-ng, and application debug code to the new wrapper.
- Moves
go-gitfrom direct to indirect dependency and documents the runtime Git requirement in the changelog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
v3/UNRELEASED_CHANGELOG.md |
Documents the switch to system Git and the new installation requirement. |
v3/internal/git/git.go |
Adds the system git CLI wrapper used by migrated callers. |
v3/internal/git/git_test.go |
Adds unit tests for wrapper success and missing-Git behavior. |
v3/internal/commands/init.go |
Replaces go-git repository initialization/staging with wrapper calls. |
v3/internal/templates/templates.go |
Replaces remote template cloning with wrapper-based cloning. |
v3/internal/doctor/doctor.go |
Uses wrapper-based HEAD hash lookup for local Wails replacements. |
v3/pkg/doctor-ng/doctor.go |
Uses wrapper-based HEAD hash lookup for local Wails replacements. |
v3/pkg/application/application_debug.go |
Uses wrapper-based HEAD hash lookup in debug startup logging. |
v3/go.mod |
Moves go-git to the indirect dependency block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil { | ||
| return "", err | ||
| } | ||
| return hash[:8], nil |
| if isNotFound(err) { | ||
| return ErrNotInstalled | ||
| } | ||
| return fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, bytes.TrimSpace(out)) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
v3/internal/git/git_test.go (1)
90-96: ⚡ Quick winAssert that
Clone(..., tag)actually checks out the requested ref.This test currently verifies only “no error”. Add a post-clone
HeadHashcomparison (orgit rev-parse) against the source tag commit so the tag-checkout contract is explicitly protected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/git/git_test.go` around lines 90 - 96, The test TestClone_WithTag only asserts Clone(src,dst,"v1.0.0") returns no error but doesn't verify the checked-out ref; update the test to resolve the source tag commit and the clone's HEAD and assert they match (e.g., call the repository helper that returns the tag commit hash or run git rev-parse on src for "refs/tags/v1.0.0" and on dst for "HEAD"/use HeadHash helper) so Clone(src,dst,"v1.0.0") is validated to have checked out the tag commit.v3/internal/doctor/doctor.go (1)
81-83: Use correct git hash truncation; review the..directory assumption for git-root lookup
git.HeadHash(dir)already returns the short 8-character hash (hash[:8]), so appendinghashverbatim is correct.filepath.Join(wailsPackage.Replace.Path, "..")assumesReplace.Pathpoints at the module directory (here,v3/go.mod) so the parent is the git root; that fits this repo layout. For non-standard local replace layouts, the suffix will just be omitted (errors are ignored). Optionally try bothReplace.Pathandfilepath.Dir(Replace.Path)/ detect the actual git root for robustness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/doctor/doctor.go` around lines 81 - 83, The code currently appends the git hash returned by git.HeadHash to wailsVersion but to be robust: rely on git.HeadHash’s short-8 behavior (do not re-truncate) and broaden the lookup for the git root by calling git.HeadHash on both wailsPackage.Replace.Path and, if that fails, on filepath.Dir(wailsPackage.Replace.Path) (or otherwise detect the git root) before appending the suffix; reference git.HeadHash and wailsPackage.Replace.Path (and filepath.Dir) when making this change so you try the module path and its parent fallback and only append the returned hash when err == nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/internal/git/git_test.go`:
- Line 36: TestInit_Error uses a hardcoded absolute path that can be
platform-flaky; change the test to construct a guaranteed-missing path using the
test's temporary directory (t.TempDir()) and pass that to Init so the path is
OS-agnostic and reliably absent; update the TestInit_Error test to create a
non-existent subpath under t.TempDir() (e.g. filepath.Join(t.TempDir(),
"nonexistent_subdir")) and assert Init(...) returns an error.
In `@v3/internal/git/git.go`:
- Around line 25-26: Return includes raw git args which can leak credentials;
sanitize any URL auth before embedding args in error messages. Implement a
helper (e.g., sanitizeGitArgs or redactAuthFromURL) that iterates over args and
for any token of the form scheme://user@host or containing userinfo
removes/replaces the userinfo (or replaces entire auth segment with "<redacted>"
using url.Parse and clearing User) and use that sanitized join in the fmt.Errorf
call instead of strings.Join(args, " "). Apply this change to the error returns
that reference args (including the occurrences around Clone and RemoteAdd).
- Line 47: The code slices hash with hash[:8] which can panic if hash is shorter
than 8 bytes; in the function that currently returns "hash[:8], nil" (look for
the function in git.go that constructs/returns variable named hash) add a guard:
check len(hash) >= 8 and if not return an error (e.g., fmt.Errorf("short git
hash: %d bytes", len(hash))) instead of slicing, otherwise return the 8-byte
prefix; add fmt import if necessary.
In `@v3/internal/templates/templates.go`:
- Around line 222-236: The gitclone function currently creates a temp dir with
os.MkdirTemp but returns that path when git.Clone fails, leaving orphaned
directories; update gitclone so that after calling git.Clone(url, dirname, tag)
any non-nil error triggers cleanup of the temp directory (os.RemoveAll(dirname))
before returning the error (and do not return the dirname on error), keeping the
existing parsing of uri/tag and preserving normal success behavior.
- Around line 229-233: The current parsing splits on the first "@" and breaks
SCP-style SSH URLs; change it to split on the last "@" instead by using
strings.LastIndex(uri, "@") to derive url and tag (url = uri[:i], tag =
uri[i+1:]) and only treat the suffix as a ref when it doesn’t look like an SCP
host/path (e.g., if tag contains ":" or otherwise looks like host:path treat it
as part of the URL and set tag = ""), updating the variables used later in
templates.go (the uri/url/tag parsing logic) accordingly.
---
Nitpick comments:
In `@v3/internal/doctor/doctor.go`:
- Around line 81-83: The code currently appends the git hash returned by
git.HeadHash to wailsVersion but to be robust: rely on git.HeadHash’s short-8
behavior (do not re-truncate) and broaden the lookup for the git root by calling
git.HeadHash on both wailsPackage.Replace.Path and, if that fails, on
filepath.Dir(wailsPackage.Replace.Path) (or otherwise detect the git root)
before appending the suffix; reference git.HeadHash and
wailsPackage.Replace.Path (and filepath.Dir) when making this change so you try
the module path and its parent fallback and only append the returned hash when
err == nil.
In `@v3/internal/git/git_test.go`:
- Around line 90-96: The test TestClone_WithTag only asserts
Clone(src,dst,"v1.0.0") returns no error but doesn't verify the checked-out ref;
update the test to resolve the source tag commit and the clone's HEAD and assert
they match (e.g., call the repository helper that returns the tag commit hash or
run git rev-parse on src for "refs/tags/v1.0.0" and on dst for "HEAD"/use
HeadHash helper) so Clone(src,dst,"v1.0.0") is validated to have checked out the
tag commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b4e4d6e-c8f3-494d-85d1-e9f2e276716d
📒 Files selected for processing (9)
v3/UNRELEASED_CHANGELOG.mdv3/go.modv3/internal/commands/init.gov3/internal/doctor/doctor.gov3/internal/git/git.gov3/internal/git/git_test.gov3/internal/templates/templates.gov3/pkg/application/application_debug.gov3/pkg/doctor-ng/doctor.go
|
|
||
| func TestInit_Error(t *testing.T) { | ||
| // git -C on a non-existent path fails | ||
| if err := Init("/nonexistent_wails_test_path_xyz"); err == nil { |
Contributor
There was a problem hiding this comment.
Use an OS-agnostic missing path in TestInit_Error.
"/nonexistent_wails_test_path_xyz" is platform-specific and can be flaky. Build a guaranteed-missing path under t.TempDir() instead.
Suggested fix
func TestInit_Error(t *testing.T) {
// git -C on a non-existent path fails
- if err := Init("/nonexistent_wails_test_path_xyz"); err == nil {
+ missing := filepath.Join(t.TempDir(), "does-not-exist")
+ if err := Init(missing); err == nil {
t.Fatal("expected error for nonexistent path")
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := Init("/nonexistent_wails_test_path_xyz"); err == nil { | |
| func TestInit_Error(t *testing.T) { | |
| // git -C on a non-existent path fails | |
| missing := filepath.Join(t.TempDir(), "does-not-exist") | |
| if err := Init(missing); err == nil { | |
| t.Fatal("expected error for nonexistent path") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/internal/git/git_test.go` at line 36, TestInit_Error uses a hardcoded
absolute path that can be platform-flaky; change the test to construct a
guaranteed-missing path using the test's temporary directory (t.TempDir()) and
pass that to Init so the path is OS-agnostic and reliably absent; update the
TestInit_Error test to create a non-existent subpath under t.TempDir() (e.g.
filepath.Join(t.TempDir(), "nonexistent_subdir")) and assert Init(...) returns
an error.
Comment on lines
+25
to
+26
| return fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, bytes.TrimSpace(out)) | ||
| } |
Contributor
There was a problem hiding this comment.
Redact git arguments before embedding them in errors.
strings.Join(args, " ") can expose credentials/tokens when url contains embedded auth (e.g., https://token@host/...) in Clone/RemoteAdd failures. Return a sanitized command string instead.
Suggested fix
import (
"bytes"
"errors"
"fmt"
+ "net/url"
"os/exec"
"strings"
)
@@
+func sanitizeArg(arg string) string {
+ u, err := url.Parse(arg)
+ if err != nil || u.User == nil {
+ return arg
+ }
+ u.User = url.UserPassword("****", "****")
+ return u.String()
+}
+
+func sanitizeArgs(args []string) string {
+ out := make([]string, len(args))
+ for i, a := range args {
+ out[i] = sanitizeArg(a)
+ }
+ return strings.Join(out, " ")
+}
+
func run(args ...string) error {
out, err := exec.Command("git", args...).CombinedOutput()
@@
- return fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, bytes.TrimSpace(out))
+ return fmt.Errorf("git %s: %w\n%s", sanitizeArgs(args), err, bytes.TrimSpace(out))
}
return nil
}
@@
- return "", fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, bytes.TrimSpace(out))
+ return "", fmt.Errorf("git %s: %w\n%s", sanitizeArgs(args), err, bytes.TrimSpace(out))
}Also applies to: 36-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/internal/git/git.go` around lines 25 - 26, Return includes raw git args
which can leak credentials; sanitize any URL auth before embedding args in error
messages. Implement a helper (e.g., sanitizeGitArgs or redactAuthFromURL) that
iterates over args and for any token of the form scheme://user@host or
containing userinfo removes/replaces the userinfo (or replaces entire auth
segment with "<redacted>" using url.Parse and clearing User) and use that
sanitized join in the fmt.Errorf call instead of strings.Join(args, " "). Apply
this change to the error returns that reference args (including the occurrences
around Clone and RemoteAdd).
| if err != nil { | ||
| return "", err | ||
| } | ||
| return hash[:8], nil |
Contributor
There was a problem hiding this comment.
Guard short hash output before slicing.
hash[:8] can panic if output is unexpectedly shorter than 8 chars. Add a length check and return an error instead.
Suggested fix
func HeadHash(dir string) (string, error) {
hash, err := output("-C", dir, "rev-parse", "HEAD")
if err != nil {
return "", err
}
+ if len(hash) < 8 {
+ return "", fmt.Errorf("git rev-parse returned short hash %q", hash)
+ }
return hash[:8], nil
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/internal/git/git.go` at line 47, The code slices hash with hash[:8] which
can panic if hash is shorter than 8 bytes; in the function that currently
returns "hash[:8], nil" (look for the function in git.go that constructs/returns
variable named hash) add a guard: check len(hash) >= 8 and if not return an
error (e.g., fmt.Errorf("short git hash: %d bytes", len(hash))) instead of
slicing, otherwise return the 8-byte prefix; add fmt import if necessary.
Comment on lines
+222
to
236
| // gitclone clones uri into a temporary directory and returns its path. | ||
| func gitclone(uri string) (string, error) { | ||
| // Create temporary directory | ||
| dirname, err := os.MkdirTemp("", "wails-template-*") | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // Parse remote template url and version number | ||
| templateInfo := strings.Split(uri, "@") | ||
| cloneOption := &git.CloneOptions{ | ||
| URL: templateInfo[0], | ||
| parts := strings.SplitN(uri, "@", 2) | ||
| url, tag := parts[0], "" | ||
| if len(parts) > 1 { | ||
| tag = parts[1] | ||
| } | ||
| if len(templateInfo) > 1 { | ||
| cloneOption.ReferenceName = plumbing.NewTagReferenceName(templateInfo[1]) | ||
| } | ||
|
|
||
| _, err = git.PlainClone(dirname, false, cloneOption) | ||
|
|
||
| return dirname, err | ||
|
|
||
| return dirname, git.Clone(url, dirname, tag) | ||
| } |
Contributor
There was a problem hiding this comment.
Temp directory not cleaned up when clone fails.
If git.Clone fails, gitclone returns the temp directory path along with the error. The caller's cleanup function is defined only after gitclone succeeds, so failed clones leave orphaned temp directories.
🛠️ Proposed fix
func gitclone(uri string) (string, error) {
dirname, err := os.MkdirTemp("", "wails-template-*")
if err != nil {
return "", err
}
parts := strings.SplitN(uri, "@", 2)
url, tag := parts[0], ""
if len(parts) > 1 {
tag = parts[1]
}
- return dirname, git.Clone(url, dirname, tag)
+ if err := git.Clone(url, dirname, tag); err != nil {
+ os.RemoveAll(dirname)
+ return "", err
+ }
+ return dirname, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // gitclone clones uri into a temporary directory and returns its path. | |
| func gitclone(uri string) (string, error) { | |
| // Create temporary directory | |
| dirname, err := os.MkdirTemp("", "wails-template-*") | |
| if err != nil { | |
| return "", err | |
| } | |
| // Parse remote template url and version number | |
| templateInfo := strings.Split(uri, "@") | |
| cloneOption := &git.CloneOptions{ | |
| URL: templateInfo[0], | |
| parts := strings.SplitN(uri, "@", 2) | |
| url, tag := parts[0], "" | |
| if len(parts) > 1 { | |
| tag = parts[1] | |
| } | |
| if len(templateInfo) > 1 { | |
| cloneOption.ReferenceName = plumbing.NewTagReferenceName(templateInfo[1]) | |
| } | |
| _, err = git.PlainClone(dirname, false, cloneOption) | |
| return dirname, err | |
| return dirname, git.Clone(url, dirname, tag) | |
| } | |
| // gitclone clones uri into a temporary directory and returns its path. | |
| func gitclone(uri string) (string, error) { | |
| dirname, err := os.MkdirTemp("", "wails-template-*") | |
| if err != nil { | |
| return "", err | |
| } | |
| parts := strings.SplitN(uri, "@", 2) | |
| url, tag := parts[0], "" | |
| if len(parts) > 1 { | |
| tag = parts[1] | |
| } | |
| if err := git.Clone(url, dirname, tag); err != nil { | |
| os.RemoveAll(dirname) | |
| return "", err | |
| } | |
| return dirname, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/internal/templates/templates.go` around lines 222 - 236, The gitclone
function currently creates a temp dir with os.MkdirTemp but returns that path
when git.Clone fails, leaving orphaned directories; update gitclone so that
after calling git.Clone(url, dirname, tag) any non-nil error triggers cleanup of
the temp directory (os.RemoveAll(dirname)) before returning the error (and do
not return the dirname on error), keeping the existing parsing of uri/tag and
preserving normal success behavior.
Comment on lines
+229
to
233
| parts := strings.SplitN(uri, "@", 2) | ||
| url, tag := parts[0], "" | ||
| if len(parts) > 1 { | ||
| tag = parts[1] | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for SCP-like URL patterns in test files and documentation
rg -n 'git@[a-zA-Z0-9.-]+:' --type-add 'docs:*.md' --type=docs --type=goRepository: wailsapp/wails
Length of output: 407
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show parsing code around templates.go lines ~229-233
sed -n '200,260p' v3/internal/templates/templates.go
echo '---'
# Show gitURLToModulePath in init.go lines ~23-27 (and surrounding)
sed -n '1,120p' v3/internal/commands/init.go
echo '---'
# Search for SplitN(uri, "@", 2) and related parsing
rg -n 'SplitN\([^,]+,\s*"@",\s*2\)' -S v3/internal || true
rg -n 'LastIndex\([^,]*,\s*"@"\)' -S v3/internal || true
echo '---'
# Search for parsing "@", tag, or "@<version>" in templates/commands packages
rg -n '@\s*[vV]?[0-9]' -S v3/internal || true
rg -n 'module.*tag|tag.*module|`@v`' -S v3/internal || true
echo '---'
# Search for git@ URLs with an appended tag in tests/docs
rg -n 'git@[a-zA-Z0-9.-]+:[^ \t'\""]+@[^\s"'\"']+' -S v3/internal . || trueRepository: wailsapp/wails
Length of output: 50459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show parsing code around templates.go lines ~229-233
sed -n '200,260p' v3/internal/templates/templates.go
echo '---'
# Show gitURLToModulePath in init.go lines ~23-27 (and surrounding)
sed -n '1,120p' v3/internal/commands/init.go
echo '---'
# Search for SplitN(uri, "@", 2) and related parsing
rg -n 'SplitN\([^,]+,\s*"@",\s*2\)' -S v3/internal || true
rg -n 'LastIndex\([^,]*,\s*"@"\)' -S v3/internal || true
echo '---'
# Search for parsing "@", tag, or "@<version>" in templates/commands packages
rg -n '@\s*[vV]?[0-9]' -S v3/internal || true
rg -n 'module.*tag|tag.*module|`@v`' -S v3/internal || true
echo '---'
# Search for git@ URLs with an appended tag in tests/docs
rg -n 'git@[a-zA-Z0-9.-]+:[^ \t'"']+@[^\s'"']+' -S v3/internal . || trueRepository: wailsapp/wails
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on template URI parsing and call sites
rg -n "getRemoteTemplate\(|gitclone\(" v3/internal/templates v3/internal/commands v3/internal | head -n 50
# Show templates.go around getRemoteTemplate and call sites for gitclone
sed -n '150,280p' v3/internal/templates/templates.go
# Show where uri passed into getRemoteTemplate; search for "-t" or Template flag usage
rg -n "TemplateName|remote template|sourceRemote|getRemoteTemplate" v3/internal/commands v3/internal/templates | head -n 80
# Inspect internal/git Clone
fd -t f -a "internal/git" >/dev/null 2>&1 || true
git_files=$(fd -t f "clone.go|git.go|repo.go|remote" v3/internal/git 2>/dev/null || true)
echo "git_files:"; echo "$git_files" | head
# list files
ls -la v3/internal/git || true
# print go files in v3/internal/git (small)
find v3/internal/git -maxdepth 1 -type f -name "*.go" -print
# show Clone implementation
rg -n "func\s+Clone\s*\(" v3/internal/git -S || true
rg -n "tag string" v3/internal/git -S || true
# Show git.Clone
clone_file=$(rg -n "func\s+Clone\s*\(" v3/internal/git -S --files-with-matches || true)
echo "clone_file(s):"
echo "$clone_file"
if [ -n "$clone_file" ]; then
# show content of first match file around func
f=$(echo "$clone_file" | head -n 1)
rg -n "func\s+Clone\s*\(" "$f" -n
start=$(rg -n "func\s+Clone\s*\(" "$f" | head -n 1 | cut -d: -f1)
# print around start
from=$((start-20)); to=$((start+120))
sed -n "${from},${to}p" "$f"
fi
# Search for tests specifically for templates with @ tag in scp-like URLs
rg -n "wails.*init.*-t|template.*`@v`|`@v`[0-9]" v3/internal -S --glob='*test.go' || true
rg -n "SplitN\(uri, \"@\", 2\)" v3/internal -S || true
# Search for patterns of scp-like URL with `@ref` in tests/docs (limit output)
rg -n "git@[a-zA-Z0-9.-]+:[^\"'\s]+@v[0-9]" v3 -S --glob='*.md' --glob='*test.go' || trueRepository: wailsapp/wails
Length of output: 8285
Fix remote template @ref parsing for SCP-like SSH URLs.
v3/internal/templates/templates.go uses strings.SplitN(uri, "@", 2) and treats the suffix as the git ref passed to git clone --branch. For an SSH/SCP-style template like git@github.com:user/repo.git@v1.0.0, this incorrectly sets url="git" and tag="github.com:user/repo.git@v1.0.0".
Use “split on the last @” and only accept the suffix as a ref when it’s actually a ref (e.g., the suffix shouldn’t still look like SCP host/path by containing :), rather than blindly splitting on the first @.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/internal/templates/templates.go` around lines 229 - 233, The current
parsing splits on the first "@" and breaks SCP-style SSH URLs; change it to
split on the last "@" instead by using strings.LastIndex(uri, "@") to derive url
and tag (url = uri[:i], tag = uri[i+1:]) and only treat the suffix as a ref when
it doesn’t look like an SCP host/path (e.g., if tag contains ":" or otherwise
looks like host:path treat it as part of the URL and set tag = ""), updating the
variables used later in templates.go (the uri/url/tag parsing logic)
accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
v3/internal/git: a thin, stdlib-only wrapper around the systemgitexecutable with five exported functions —HeadHash,Clone,Init,RemoteAdd,AddAllgithub.com/go-git/go-git/v5:internal/commands/init.go,internal/templates/templates.go,internal/doctor/doctor.go,pkg/doctor-ng/doctor.go,pkg/application/application_debug.gogo-git/go-git/v5moves from a direct →// indirectdependency ingo.mod(it remains as a transitive dep ofwailsapp/task/v3, which we do not control)gitis not found inPATH, all functions return the sentinelgit.ErrNotInstalledwith a helpful message pointing to https://git-scm.comBehaviour notes
git.PlainClone(dir, false, &CloneOptions{URL})git clone --quiet <url> <dir>git.PlainClonewithReferenceNamegit clone --quiet --branch <tag> <url> <dir>git.PlainInit(dir, false)git -C <dir> init --quietrepo.CreateRemote(&RemoteConfig{Name, URLs})git -C <dir> remote add <name> <url>worktree.Add(".")git -C <dir> add .git.PlainOpen(dir)+repo.Head()+hash[:8]git -C <dir> rev-parse HEAD(first 8 chars)The three diagnostic commit-hash lookups (doctor, doctor-ng, application_debug) already silently ignored errors — behaviour is unchanged; if git is not installed or the directory is not a repo, the hash is simply omitted from the version string.
Test plan
go test -cover ./v3/internal/git/...→coverage: 100.0% of statementsgo vet ./v3/internal/git/... ./v3/internal/doctor/... ./v3/internal/templates/... ./v3/internal/commands/... ./v3/pkg/doctor-ng/...→ no errorswails3 init -n myappinitialises a git repo correctly when--git <url>is passedwails3 init -n myapp -t <remote-url@tag>clones the tagged template correctlywails3 doctoragainst a local replace-directive path shows the 8-char commit hashgitin PATH:wails3 init --git ...returns a clear error containing "git is not installed"Summary by CodeRabbit
Changed
gitcommand-line interface instead of using a library dependency. Git must be present in your PATH.Tests